Skip to content

Conversation

@flohw
Copy link
Contributor

@flohw flohw commented Dec 22, 2023

Hello!

As a Christmas gift I come here to preset the work to hopefully close #67 properly 🎅 🥳

I added the cases suggested by @0x346e3730 based on the existing test cases of @raneomik.

Let me know if the new algorithm is ok for you.
The regexes are a bit stricter as there is with data set into, that's why I return early on line 117. It make the regexes a bit easier to read I think. That's not perfect to my eyes (depending on a static string is not algorithmically valid to me) but it may be enough. WDYT?
I will also create named group to make the code clearer.

I didn't set cases stated by er1z in the issue as he didn't explain much about its problem.

If you look at it during this long weekend, please don't merge. I wanted to sign my commits but I have some issue with my passphrase memory 😅

Thanks and have a nice week-end!

[EDIT]: Failing test seems unrelated.

@kbond
Copy link
Member

kbond commented Dec 22, 2023

Awesome, thanks for working on this @flohw! Let me know when it's ready and I'll merge!

@kbond kbond marked this pull request as draft December 22, 2023 18:53
@flohw
Copy link
Contributor Author

flohw commented Dec 23, 2023

Hi @kbond,

Chocolate activates some neurons! I added the regex group names to be more comprehensible. Also I updated the preg_replace to replace multiple non-word characters by a single hyphen to shorten the filename a bit.

For example for the dataset single quote for array index access with {{[\'id\']|filter(\'system\')}}st as a dataset name, the {{[ was transformed to --- in my previous version, it is a single - now.

Let me know if I need to make changes you have in mind.

Have a nice week-end!

@flohw flohw marked this pull request as ready for review December 23, 2023 09:07
@kbond kbond merged commit 7144c6a into zenstruck:1.x Dec 23, 2023
@kbond
Copy link
Member

kbond commented Dec 23, 2023

Thanks @flohw!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Undefined array key 1 when the test failed

2 participants